Refuse tokens that carry no groups claim - #17
Merged
Conversation
The workspace is gated in two places on purpose: Authentik binds the
application to the dropbox group, and the app re-checks the groups claim. The
second check existed to not depend on the first — but when the claim was
absent it fell back to trusting the first, which is the one case where the
backstop was the only thing left to do its job.
Absence of the claim does not mean the user belongs to nothing. It means the
provider's configuration has drifted, and the honest answer to "is this user a
member" is that we cannot tell.
Checked the live provider before changing the default, rather than assuming:
- a "Beckham Share - OIDC groups" scope mapping is bound to the provider and
returns {"groups": [g.name for g in user.ak_groups.all()]}
- evaluated against both accounts it yields a populated list containing
dropbox
- the app requests the groups scope in the authorization request
- the current log window covers four sign-ins and no fallback warnings
So nobody is relying on the fallback and nobody gets locked out.
ALLOW_MISSING_GROUPS_CLAIM (default false) keeps a way back in if that mapping
ever breaks, since locking the only two members out of their own files while
repairing an identity provider is its own kind of outage. It re-opens the gap
it exists to close, so the app warns about it at startup for as long as it is
set.
# Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
The workspace is gated in two places on purpose — Authentik binds the
application to the
dropboxgroup, and the app re-checks thegroupsclaim.The second check exists precisely so the workspace does not depend on the first.
But when the claim was absent, it fell back to trusting the first — in the one
case where the backstop was the only thing left to do its job.
Absence of the claim does not mean the user belongs to nothing. It means the
provider's configuration has drifted, and the honest answer to "is this user a
member?" is that we cannot tell. So we stop saying yes.
Checked before changing the default
Locking the only two members out of their own file store would be a poor way to
discover the mapping was broken, so this was verified against the live provider
rather than assumed:
Beckham Share - OIDC groupsscope mapping is bound to the provider,returning
{"groups": [group.name for group in request.user.ak_groups.all()]}.dropbox.groupsscope — confirmed in the authorization requestthe live provider accepts.
Nobody is relying on the fallback, and existing signed-in sessions carry the
claim, so no one is logged out by this.
The escape hatch
ALLOW_MISSING_GROUPS_CLAIM, defaultfalse.Kept deliberately. If that scope mapping ever breaks — an Authentik upgrade, a
fat-fingered edit — the failure mode without a lever is "both members locked out
of their own files while someone repairs an identity provider under pressure".
That is its own kind of outage.
It re-opens the gap it exists to close, so it is not quiet about it: the app
logs a warning at startup for as long as it is set, and again on every
admitted request.
docs/SECURITY.mdsays to clear it once the mapping is fixed.Verification
Backend suite: 28 tests pass, including three new ones — a groupless token
is refused at
/app, is refused by the management API, and is admitted onlywhen the flag is deliberately set.